builder-utils: Rewrite locale migration to use fd-based traversal#733
Conversation
dd96137 to
5dffdab
Compare
|
I still need to test this and find more examples of how locale directories are structured by apps. The previous code was recursively copying everything inside the localedir which isn't possible to do when this is fd based. I don't think we can support the exact same behaviour. I managed to support the gettext localedir structure and the when locale files are inside the immediate locale directory. AppStream also mainly handles these two structures too. I don't think other structures exists but who knows what some random software buildsystem is doing. If it can't be fully emptied, it cannot be removed or symlinked, then it gets left in a weird state and errors from unlinkat or symlinkat cannot sanely be ignored too. |
|
Ok maybe this is closer to the previous behaviour, did something to recursively migrate things. |
|
Needs a rebase over other PRs, but I think it's ok now, minus me testing it some more. |
d1fdef0 to
a55dfda
Compare
|
it would be nice to have reviews on this. |
|
I emailed @alexlarsson a while back about this but haven't heard back. It would be nice to get an extra look at this from him. |
|
|
||
| glnx_gen_temp_name (tmp_name); | ||
|
|
||
| if (!glnx_renameat (source_dfd, name, source_dfd, tmp_name, error)) |
There was a problem hiding this comment.
I'm sligtlty worried about this, as there is no guarantees that the generated tmp name doesn't already exist as an empty dir which would make the renameat not error. However, I have not been able to come up with some case where this could end up causing problems. However, in some ideal world we would pass RENAME_NOREPLACE here.
There was a problem hiding this comment.
However, in some ideal world we would pass RENAME_NOREPLACE here.
there is glnx_renameat2_noreplace which we can use I guess but I too find it unlikely... also empty directories shouldn't cause problem here even if someone managed to place one.
|
lgtm |
|
I have deployed this to Flathub just now, will try to backport based on the feedback from there in a couple of days. |
No description provided.